Adapt to cat_tools 0.3.0: renamed function, two new object types - #28
Open
jnasbyupgrade wants to merge 2 commits into
Open
Adapt to cat_tools 0.3.0: renamed function, two new object types#28jnasbyupgrade wants to merge 2 commits into
jnasbyupgrade wants to merge 2 commits into
Conversation
pgxn install --unstable cat_tools resolves to the newest release actually published to the PGXN package index, which is still 0.2.1 (2017) and fails standalone on modern PostgreSQL with "column oid specified more than once" at CREATE EXTENSION. A fixed release, 0.3.0, is tagged in cat_tools' own git repo but hasn't been uploaded to PGXN yet, so the Makefile's cat_tools target now clones Postgres-Extensions/cat_tools at the 0.3.0 tag and builds/installs it directly. Since this is the first time object_reference's suite has actually run against a real, working cat_tools, two small fallout fixes are needed: - cat_tools.function__arg_types_text() is deprecated in 0.3.0 in favor of cat_tools.routine__parse_arg_types_text() (identical signature/body, just renamed, deprecated one emits a WARNING on every call). Switched object_reference's one call site to the non-deprecated name. - cat_tools 0.3.0's object_type enum grew two new members, "partitioned table" and "partitioned index". pg_get_object_address() doesn't recognize either (only the base table/index types they derive from), so object_reference classifies them as unsupported, matching object_reference.unsupported()'s existing handling of "event trigger" for the same reason. test/sql/all.sql's sanity-check of the unsupported set is updated to match. sql/object_reference--stable.sql and test/expected/zzz_build.out are regenerated (make results) to match. Extracted from PR Postgres-Extensions#5, which had scope-crept into also carrying this fix alongside the actual CI/pgxn-tools migration; splitting it out here so it can be reviewed and merged independently.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jnasbyupgrade
added a commit
that referenced
this pull request
Aug 7, 2026
Rebuilt fresh on top of current master, containing only the actual CI/pgxn-tools migration (the SQL style linter integration and the cat_tools 0.3.0 dependency fix that had scope-crept into this branch are split out to #16 and #28 respectively). - Add .github/workflows/ci.yml: a `changes` job (docs-only gate + PG-major-matrix derivation from two constants), a `test` matrix job (container: pgxn/pgxn-tools, PostgreSQL 12-18), and an `all-checks-passed` aggregation gate for use as a single stable required status check. - Remove .travis.yml and pg-travis-test.sh, superseded by the above. - test/dump/run.sh: add -X to several psql invocations, disabling ~/.psqlrc so test runs are deterministic.
…ls 0.3.0 pgxn install --unstable cat_tools now resolves to cat_tools 0.3.0 directly (confirmed against the live PGXN index and by a clean CREATE EXTENSION cat_tools; both locally and, once pushed, in the actual object_reference CI run for Postgres-Extensions#5's slimmed CI-migration branch -- it went fully green without this Makefile change at all). The PGXN package index being stuck at the broken, 2017-era 0.2.1 release was true when this fix was first written, but isn't true anymore, so the git-clone-from-tag workaround has nothing left to work around. Keeping it would leave a Makefile comment describing a problem that no longer exists. The SQL/test fixes (renamed function call, new object_type enum members classified as unsupported) are unaffected -- those are needed regardless of how cat_tools 0.3.0 gets installed.
jnasbyupgrade
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extracted from #5 ("Switch to pgxn-tools based testing"), which had
scope-crept into carrying this fix alongside the actual CI/pgxn-tools
migration. Splitting it out so it can be reviewed and merged on its own.
Update while preparing this PR: it originally also changed the
Makefile'scat_toolstarget to build from cat_tools'0.3.0git tagdirectly, working around PGXN's package index being stuck on the broken,
2017-era
0.2.1release (column "oid" specified more than onceatCREATE EXTENSION cat_tools). That's no longer true -- PGXN's index nowserves
0.3.0directly (confirmed against the live index, a cleanCREATE EXTENSION cat_toolslocally, and #5's now-green CI, all withoutany Makefile change). So this PR no longer touches the
Makefileat all;plain
pgxn install --unstable cat_toolsalready resolves to0.3.0.What's left: adapting to cat_tools 0.3.0's real API/enum changes
Because cat_tools has apparently never successfully run against
object_reference's full suite before (whatever version was previously
resolved), this surfaced two small, real fixes:
cat_tools.function__arg_types_text()is deprecated in 0.3.0 in favorof
cat_tools.routine__parse_arg_types_text()(identicalsignature/body, just renamed) and emits a
WARNINGon every call.Switched
sql/object_reference.sql's one call site to thenon-deprecated name.
object_typeenum grew two new members,partitioned tableandpartitioned index.pg_get_object_address()doesn't recognize either (only the base
table/indextypes theyderive from), so they're classified as unsupported
(
object_reference.unsupported()), matching the existing handling ofevent triggerfor the same reason.test/sql/all.sql's sanity-checkof the unsupported set is updated to match.
sql/object_reference--stable.sql(generated fromsql/object_reference.sql)and
test/expected/zzz_build.outare regenerated to match.Verification
CREATE EXTENSION cat_tools;standalone on PostgreSQL 17 via plainpgxn install --unstable cat_tools: installs cleanly at 0.3.0.make teston PostgreSQL 17: all 7 tests pass cleanly, zero diffs.test/expected/zzz_build.out's update here is a pure mechanicalline-number shift (from the added enum-growth comment) plus the
disappearance of the deprecated-function warning -- matches exactly
what's already landed and CI-verified on
master-pre-rollback(thebranch Switch to pgxn-tools based testing #5 used to also carry this content on).
No longer anything for #5 to depend on merging first -- #5's CI already
passes cleanly on its own, since PGXN already serves a working cat_tools.
This PR is now a plain correctness/adaptation fix, reviewable and
mergeable independently and in either order relative to #5.